-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bump netlify/next runtime and fix api routes in middleware #13040
Conversation
✅ Deploy Preview for ethereumorg ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When navigating to a non-existing nested page like /en/dapps/test
we enter in a redirect loop.
…edirect loop with some nested pages
@@ -48,7 +51,7 @@ export async function middleware(req: NextRequest) { | |||
const localeDetected = detectLocale(req.headers.get("accept-language")) | |||
const locale = localeDetected || DEFAULT_LOCALE | |||
|
|||
const redirectUrl = new URL(`/${locale}${pathname}`, req.url) | |||
const redirectUrl = new URL(`/${locale}${pathname}${search}`, req.url) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding the search
params to the redirect, thing that was missing.
Description
Things done in this PR:
@netlify/plugin-nextjs
to the latest, v5included_files
fromnetlify.toml
doesn't seem to work, we excluded the heavy files from Netlify function bundles in thenext.config.js
file/api
routes from middlewareBonus:
This also fix the issue we currently have when navigating to the home page by using the eth logo on the navbar where it was doing a full page nav instead of a client side one.